Narrow type for Extension::getConfiguration if class exists #384
Conversation
RobertMe
force-pushed
the
configuration-type
branch
2 times, most recently
from
577564f to
93f1df9
Compare
| public function load(array $configs, ContainerBuilder $container): void | ||
| { | ||
| \PHPStan\Testing\assertType( | ||
| 'PHPStan\Type\Symfony\Extension\WithConfiguration\Configuration', |
There was a problem hiding this comment.
You could do Configuration::class here.
There was a problem hiding this comment.
Will do. I think I based it on some other test fixture, which also used a string. But if ::class works and is preferred I will obviously change it 👍🏻
| return new NullType(); | ||
| } | ||
|
|
||
| // TODO: return NullType if configuration class has constructor with required arguments |
There was a problem hiding this comment.
Please solve this todo, or remove it.
There was a problem hiding this comment.
If you can provide some pointers on how to determine this I will try to implement it :) I haven't been able to find some example on how to determine / inspect such information.
There was a problem hiding this comment.
You can get ClassReflection from ReflectionProvider and ask about constructor from there.
There was a problem hiding this comment.
That was the easy part which I did find out already :) Anyway, I'm now iterating over the variants => parameters => checking isOptional on every one of them. Not sure if there is an easier way to do this though.
| ): ?Type | ||
| { | ||
| $extensionType = $scope->getType($methodCall->var); | ||
| $classes = $extensionType->getObjectClassNames(); |
There was a problem hiding this comment.
We can solve this for union types too. Don't return early if it's not 1, iterate over the class names and create a union.
There was a problem hiding this comment.
True. But I don't think it's common. I believe that ->getConfiguration() is only called on $this (/by the extension itself), or by the Symfony framework. So my reasoning was that handling it for other cases (/unions) makes the code more complex while not being of any benefit.
But I'll update the code so it handles that as well 👍🏻
The base extension class automatically creates a Configuration instance
when a Configuration class exists in the namespace of the extension. But
PHPStan obviously doesn't understand this behaviour and always assumes
that `getConfiguration()` returns `ConfigurationInterface|null` meaning
that the default pattern to get and parse the configuration reports an
error.
I.e.:
```php
namespace Foo;
class SomeExtension extends Extension
{
public function load(array $configs, ContainerBuilder $container): void
{
$configuration = $this->getConfiguration($configs, $container);
$config = $this->processConfiguration($configuration, $configs);
}
}
```
results in an error because `processConfiguration()` doesn't accept
`ConfigurationInterface|null`. But when a `Configuration` class exists
in the same namespace as the `Extension` class (so `Foo\Extension`) an
instance of it is returned.
This `DynamicReturnTypeExtension` overrides the return type of
`Extension::getConfiguration()` so it automatically narrows the return
type in case `getConfiguration()` is not overriden and a `Configuration`
class exists. So that in the given example `getConfiguration()` doesn't
return `ConfigurationInterface|null` anymore but `Foo\Configuration` and
there is no error on calling `processConfiguration()`.
RobertMe
force-pushed
the
configuration-type
branch
from
93f1df9 to
5d1f883
Compare
|
Thank you! |
2 participants
The base extension class automatically creates a Configuration instance when a Configuration class exists in the namespace of the extension. But PHPStan obviously doesn't understand this behaviour and always assumes that
getConfiguration()returnsConfigurationInterface|nullmeaning that the default pattern to get and parse the configuration reports an error.I.e.:
results in an error because
processConfiguration()doesn't acceptConfigurationInterface|null. But when aConfigurationclass exists in the same namespace as theExtensionclass (soFoo\Extension) an instance of it is returned.This
DynamicReturnTypeExtensionoverrides the return type ofExtension::getConfiguration()so it automatically narrows the return type in casegetConfiguration()is not overriden and aConfigurationclass exists. So that in the given examplegetConfiguration()doesn't returnConfigurationInterface|nullanymore butFoo\Configurationand there is no error on callingprocessConfiguration().